Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improves assert_delivered_email and adds assert_delivered_with #228

Merged
merged 3 commits into from
Dec 13, 2016

Conversation

jbernardo95
Copy link
Contributor

Resolves #74.

@optikfluffel
Copy link

Any updates on that one? 😸

@jbernardo95
Copy link
Contributor Author

@paulcsmith ^

@jbernardo95
Copy link
Contributor Author

@optikfluffel I guess this is related with #233, am I right ?

@@ -146,7 +144,7 @@ defmodule Bamboo.Test do
Checks whether an email was delivered.

Must be used with the `Bamboo.TestAdapter` or this will never pass. In case you
are delivering from another process, the assertion waits up to 100ms before
are delivering from another process, the assertion waits up to 150ms before
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a reason for changing this to 150ms?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the time I ran the tests, 100ms seemed to not be enough. But I ran the tests now 3 times in a row with the 100ms and they seem fine.

Changed back to 100ms.

error in [ExUnit.AssertionError] ->
assert error.message =~ "0 emails delivered"
else
_ -> flunk "assert_delivered_email should failed"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should have failed? both here and in the next test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, typos.

@paulcsmith
Copy link
Contributor

@jbernardo95 Thanks for tackling his. Just a few small comments and then I'll merge this in!

Also, I think that feature is somewhat related, but I would probably test token generation in a unit test. It's easier to isolate. Then you could use this new function you wrote to just test the subject and skip the body in integration tests

after
@timeout -> flunk_with_email_list(email)
@doc """
Checks whether an email its params equal to the ones provided.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more small typo: Check whether an email's params are equal to the ones provided

@jbernardo95
Copy link
Contributor Author

@paulcsmith Updated !
CircleCI seems to be stuck...

@paulcsmith
Copy link
Contributor

@jbernardo95 Very strange! I checked Circle and it passed, bit it isn't reporting to GitHub. Can you squash your commits and force push. Maybe that'll force it to reconnect

@jbernardo95
Copy link
Contributor Author

@paulcsmith Done, let's see if this helps.

@jbernardo95
Copy link
Contributor Author

@paulcsmith It didn't help, this seems to be a CircleCI <> GitHub issue.

The other new PR I submitted isn't getting any status either. Checked back at CircleCI and it seems to not be getting GitHub notifications of new code.

@paulcsmith
Copy link
Contributor

@jbernardo95 I'll have to figure this out some other time. It appears to be working fine though so I'll merge this in. Thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants